Pull Request Test Coverage Report for Build 5795969658

Details

• 34 of 40 (85.0%) changed or added relevant lines in 4 files are covered.
• 25 unchanged lines in 5 files lost coverage.
• Overall coverage increased (+0.2%) to 64.32%

Totals
Change from base Build 5715994248:	0.2%
Covered Lines:	4741
Relevant Lines:	7371

💛 - Coveralls

@kimakan did you add some docs about this feature in docs repo?

@kimakan did you add some docs about this feature in docs repo?

The documentation will be added after the merge.

add condition possibility on visibility of message, based on preprogrammed filters.

Usecase: show user with consent == False that they need to accept Term of Use.

add condition possibility on visibility of message, based on preprogrammed filters.

Usecase: show user with consent == False that they need to accept Term of Use.

I came up with a possible solution for this that allows adding new conditions/filters easily.

The filters/conditions are defined in contact/filters.py

   class MessageFilter:

    CHOICES = (
        ("no_filter", "Show to all users"),
        ("user_has_not_consented", "Show to user who has not consented yet"),
    )

    def no_filter(request):
        return True

    def user_has_not_consented(request):
        if request.user.is_authenticated:
            if not request.user.profile.consent:
                return True
        return False                                                    

New conditions can be added directy as methods of the class MessageFilter. Every method must have a single parameter request and the name of the method should be added to CHOICES (otherwise, it will not be added to the model).

The AnnouncementMessage model stores the name of the filter/condition method (it can be selected in Django-Admin)

...
visibility_filter = models.CharField(max_length=30, choices=MessageFilter.CHOICES, default="no_filter")
def get_filter(self):
    return getattr(MessageFilter, str(self.visibility_filter))

...

The actual filtering of the messages happens in the templatetags

@register.inclusion_tag("contact/announcements.html", takes_context=True)
def show_announcements(context):
   request = context["request"]
   announcements = AnnouncementMessage.objects.filter(visible=True)
   announcements = [msg for msg in announcements if msg.get_filter()(request) is True]
   return {
       "announcements": announcements,
   }

This solution requires makemigrations after adding a new filter/condition method in the MessageFilter since new choices are added to the field of the model.
@agy-why What do you think? Is this solution worth pursuing?

I think that is exactly what we need, the makemigration is obvious and not that of an issue. Good job!

Hi @kimakan @agy-why sorry for interupt, running makemigrations on a daiquiri app would create migrations in the daiquiri repo (which is probably only installed by pip in a virtual env). This would break the way daiquiri is set up. (Unless you changed that, I am not up to date anymore, but just saw the email.)

@jochenklar that is a good feed back, thanks! I think we will not add the filtering app wise but globally on daiquiri, but it worth discussing.

If you want choices to be flexible you can implement them like
‎LICENSE_CHOICES‎: a CharField without choices in the model and choices in the admin.py and in the serializer.

@jochenklar , Thank you for the feedback! I have implemented the announcement messages following your suggestion.

Now, the new conditions do not require a migration. The default MessageFilter can be overloaded by the app and set by the setting ANNOUNCEMENT_MESSAGE_FILTER.

A new filter can be added as following (i.e. in the filters.py of the app)

from daiquiri.contact.filters import DefaultMessageFilter

class MyAppMessageFilter(DefaultMessageFilter):

    CHOICES = DefaultMessageFilter.CHOICES + (
        ("custom_condition", "Show to the custom group of users"),
    )

    def custom_condition(request):
        return True

And then set the new message filter in the settings with

ANNOUNCEMENT_MESSAGE_FILTER = 'daiquiri.myapp.filters.MyAppMessageFilter'

The current CI tests are failing because the settings from the contact app are not imported by the daiquiri app yet. It will be changed after the merge.